Skip to content

Remove mention of 'declarative' schema changer' #19443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 25, 2025

Conversation

rmloveland
Copy link
Contributor

@rmloveland rmloveland commented Mar 11, 2025

Fixes DOC-8693

Summary:

  • Remove mentions of "declarative schema changer" (DSC) for the following reasons:

    1. It is enabled by default, and 99% of users will never change the relevant setting or even need to know what that is.

    2. Its existence is an internal implementation detail that is fine for internal TSE runbooks to refer to, but should not really be in customer-facing docs since "it uses two different implementations on the inside, one of which we're migrating away from" should not be a thing that matters to 99% of users unless there is a specific problem they are working with TSE to solve.

Copy link

github-actions bot commented Mar 11, 2025

Files changed:

Copy link

netlify bot commented Mar 11, 2025

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit b71c04f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/680bceb9101e2c000845238a

Copy link

netlify bot commented Mar 11, 2025

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit b71c04f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/680bceb96fd00800088d8d66

@rmloveland rmloveland requested review from rafiss and dikshant March 11, 2025 18:07
@rmloveland
Copy link
Contributor Author

rmloveland commented Mar 11, 2025

hi @rafiss and @dikshant !

this PR comes out of a convo with Dikshant re: us maintaining lists of "which statement is done by the DSC" is not really helpful for users and is more of an internal detail that is leaking out IMO

the PR only removes 'declarative schema changer' stuff from prose, it does not remove the documentation for the variable that controls on/off

it also removes the whole "list of which statements" which, as a user, it's not clear what that list is for or why i should care other than "it's magically better somehow" which feels very internal to the team/CRL support, not user-facing

also it only changes v25.1 for our review, once we're happy with this change i will backport this change to earlier versions. If you have opinions about which earlier versions we should/should not make this change to, i'm def open to that. i was thinking v23.1+ but please let me know

Copy link

netlify bot commented Mar 11, 2025

Netlify Preview

Name Link
🔨 Latest commit b71c04f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/680bceb9475867000804b3ef
😎 Deploy Preview https://deploy-preview-19443--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this cleanup!

Fixes DOC-8693

Summary:

- Remove mentions of "declarative schema changer" (DSC) for the
  following reasons:

  - 1. It is enabled by default, and 99% of users will never change the
    relevant setting or even need to know what that is.

  - 2. Its existence is an internal implementation detail that is fine
    for internal TSE runbooks to refer to, but should not really be in
    customer-facing docs since "it uses two different implementations on
    the inside, one of which we're migrating away from" should not be a
    thing that matters to 99% of users unless there is a specific
    problem they are working with TSE to solve.
@rmloveland rmloveland force-pushed the 20250310-DOC-8693-remove-declarative-schema-changer branch from f3394f5 to 9c5ecfe Compare April 17, 2025 15:08
@rmloveland rmloveland requested a review from taroface April 17, 2025 15:08
Copy link
Contributor

@taroface taroface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just one question.

@rmloveland rmloveland enabled auto-merge (squash) April 25, 2025 18:02
@rmloveland rmloveland merged commit cb51eaf into main Apr 25, 2025
6 checks passed
@rmloveland rmloveland deleted the 20250310-DOC-8693-remove-declarative-schema-changer branch April 25, 2025 18:13
@sweethuman
Copy link

sweethuman commented Apr 26, 2025

I'm a nobody, Normie user. Honestly, I was reading this doc before this change, and after moving to 25.1 some of my migrations were failing(in a local dev machine tbh, since in prod they're already applied), and that section in the docs helped me understand better what was going on and what I would need to change to my system and/or enable or disable in my cluster. I think it's helpful to be kept in there.

ERROR: ALTER COLUMN TYPE is only implemented in the declarative schema changer

I was getting this error and with that text in there I was able to find the information, read it and understand.

With this removal, I feel like for users like me, who might go through a similar issue, it will be harder for them to figure out what went wrong.

Once you have fully migrated to DSC, it might not matter as a term as much, but as you're in the process of migrating to it, and some issues will arise for some users, it's important and helpful for them to know.

I'm gonna keep reading the docs in the wayback machine.

If you're curious how this information helped me, read this comment: prisma/prisma#26864 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants